Spanish translation #1130

emlys · 2022-12-05T23:58:06Z

Description

This PR introduces our first translation of InVEST into Spanish:

expected tasks

Add the Spanish message catalogs
Remove the logic we used to hide the workbench language setting in production mode
Uncomment the code that builds message catalogs as part of the setup.py build process

bug fixes

Refresh the app when the language setting changes to ensure that all content displays in the new language. We decided that users will have to refresh the app for the language setting to take effect. It was more complicated than it's worth to make sure that every affected component re-renders in a new language. In a future PR, I'll add a feature to prompt the user to choose a language when first opening the app.
Fix a couple of module reloads in ui_server.py
I'm not sure exactly why, but the small change to module reloading in ui_server.py triggered a weird bug where the test suite would hang indefinitely, on unrelated tests, and only when running most of the tests all together (could not reproduce with an individual test suite). It was fixed by moving the unit registry definition to its own module so that u isn't redefined when we reload spec_utils.
While debugging that, I noticed that 3 test suites (CBC, CV, and translation) imported the module under test at the top level. That meant that any import errors would crash the whole pytest command, rather than record as a test failure. I moved those imports into each individual test like we do for other models.

small enhancement

We now get the list of supported languages and their localized names from the backend, rather than defining them in the workbench.

notes

I tried to test that the page reloads when the language is changed, following this example, but it didn't work and I didn't think it was that important.

There is also a PR in the works for the user's guide translation, but I don't think those need to be linked to this PR.

Checklist

Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
~~Updated the user's guide (if needed)~~
~~Tested the affected models' UIs (if relevant)~~

…ask/spanish

phargogh

👍 🎉

phargogh · 2022-12-19T18:40:30Z

MANIFEST.in

@@ -5,3 +5,4 @@ prune ci
 prune exe
 prune installer
 prune workbench
+include src/natcap/invest/internationalization/locales/*/LC_MESSAGES/messages.mo


I didn't know we could include globs here! Very cool.

phargogh · 2022-12-19T18:44:02Z

src/natcap/invest/internationalization/locales/es/LC_MESSAGES/messages.po

-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:192
-msgid ""
-"Use user-defined local recharge data instead of calculating local "
-"recharge from the other provided data."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:195
-msgid "user-defined recharge layer (advanced)"
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:205
-msgid ""
-"Map of local recharge data. Required if User-Defined Local Recharge is "
-"selected."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:208
-msgid "local recharge"
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:212
-msgid "Use user-defined climate zone data in lieu of a global rain events table."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:215
-msgid "climate zones (advanced)"
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:222
-msgid ""
-"Climate zone ID numbers, corresponding to the values in the Climate Zones"
-" map."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:228
-msgid ""
-"The number of rain events that occur in each month in this climate zone. "
-"Replace [MONTH] with the month abbreviations: jan, feb, mar, apr, may, "
-"jun, jul, aug, sep, oct, nov, dec, so that there is a column for each "
-"month."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:236
-msgid ""
-"Table of monthly precipitation events for each climate zone. Required if "
-"User-Defined Climate Zones is selected."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:239
-msgid "climate zone table"
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:246
-msgid ""
-"Map of climate zones. All values in this raster must have corresponding "
-"entries in the Climate Zone Table."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:249
-msgid "climate zone map"
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:253
-msgid "Use montly alpha values instead of a single value for the whole year."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:256
-msgid "use monthly alpha table (advanced)"
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:264
-msgid "Values are the numbers 1-12 corresponding to each month."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:271
-msgid "The alpha value for that month."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:275
-msgid ""
-"Table of alpha values for each month. Required if Use Monthly Alpha Table"
-" is selected."
-msgstr ""
-
-#: src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py:278
-msgid "monthly alpha table"
-msgstr ""
-
+# Spanish translations for InVEST.
+# Copyright (C) 2022 Natural Capital Project
+# This file is distributed under the same license as the InVEST project.
+# FIRST AUTHOR <EMAIL@ADDRESS>, 2022.
+#
+msgid ""
+msgstr ""
+"Project-Id-Version: InVEST 3.12\n"
+"Report-Msgid-Bugs-To: jdouglass@stanford.edu\n"
+"POT-Creation-Date: 2022-10-21 13:17-0700\n"
+"PO-Revision-Date: 2022-11-19 17:42-0500\n"
+"Last-Translator: \n"


Should the Last-Translator be Patricio's email?

Sure, I don't think any of these fields are required but I'll fill in his email

phargogh · 2022-12-19T18:44:56Z

src/natcap/invest/internationalization/locales/es/LC_MESSAGES/messages.po

+msgid ""
+msgstr ""
+"Project-Id-Version: InVEST 3.12\n"
+"Report-Msgid-Bugs-To: jdouglass@stanford.edu\n"


I don't recall why my email is here ... @emlys would you like this to be your email? Or maybe we should have it email the software team email list, or else an alias at naturalcapitalproject.org?

davemfish

Thanks @emlys , this is awesome. I reviewed the whole thing.

One thing I noticed when testing it out is that the Workbench strings are not translating, and I'm not sure how that is supposed to work exactly. And strangely, there is a test that tests for that, which somehow passes.

Other than that, I just a few minor comments.

src/natcap/invest/ui_server.py

davemfish · 2023-01-05T14:28:57Z

workbench/src/renderer/app.jsx

    ipcRenderer.invoke(
      ipcMainChannels.SET_LANGUAGE, settings.language
+    ).then(() => saveSettingsStore(settings)


Functionally it's the same, but for consistency across the codebase, how do you feel about using async/await here instead of chaining the .then calls?

I realize we're not 100% consistent on this -- we tend to use .then/.catch for fetch calls such as in server_requests.js (for no particular reason), but I'd prefer to migrate towards async/await.

src/natcap/invest/ui_server.py

davemfish · 2023-01-05T15:25:31Z

workbench/tests/renderer/app.test.js

@@ -876,6 +873,7 @@ describe('Translation', () => {

  beforeAll(async () => {
    getInvestModelNames.mockResolvedValue({});
+    getSupportedLanguages.mockResolvedValue({ en: 'english', es: 'spanish' });


This is a comment about the test in this block, which didn't actually have any changes...

I think it would be best to mock window.location.reload() also. jsdom has a global window, but I see a console error saying Error: Not implemented: navigation (except hash changes) on the reload call.

Also in that test, it looks like we test that some workbench strings were translated (as opposed to invest strings) and those checks pass despite the browser refresh not happening, which I think makes sense? But if I use the workbench and change the language, I don't see those workbench strings translated. So I'm not exactly sure what's going on there.

If you're not on the latest commit, you might also be missing the message catalog for the workbench strings, hence it's defaulting to English. The translation test mocks the message catalog, so I'd expect it to pass either way.

@emlys I've confirmed that I have the latest changes, including the workbench message catalog and a fresh install, but I still can't get the workbench strings to translate. I also tried building a production version and don't get the translated strings there either. I confirmed this function is called with the correct & complete messageCatalogPath, so I'm not sure how to debug further.

function loadMessageCatalogs() { // load each language's message catalog PO file into an object // for easy access when we switch languages const languages = fs.readdirSync(`${__dirname}/../static/internationalization/locales`); if (languages) { languages.forEach((language) => { const messageCatalogPath = `${__dirname}/../static/internationalization/locales/${language}/LC_MESSAGES/messages.po`; console.log(messageCatalogPath) i18n.loadJSON(readMessageCatalog(messageCatalogPath), 'messages'); }); } logger.debug('loaded message catalogs'); }

This should be fixed now - the language code in the PO file was wrong, resulting in the messages not being used.

davemfish · 2023-01-05T15:41:29Z

I tried to test that the page reloads when the language is changed, following this example, but it didn't work and I didn't think it was that important.

I agree it's not important to test that. Though I think that basic approach would work if window.location.reload() is a mock function, which I suggested for a different reason in another comment.

emlys · 2023-01-06T21:19:27Z

Though I think that basic approach would work if window.location.reload() is a mock function

I've spent a few hours on this now and can't get it to work. The mock doesn't work, and the test doesn't pass. If it's important to mock this maybe we can debug it together, otherwise, I don't think it's worth the effort to test.

davemfish · 2023-01-09T15:15:02Z

I've spent a few hours on this now and can't get it to work. The mock doesn't work, and the test doesn't pass.

@emlys I've been there.

If it's important to mock this maybe we can debug it together, otherwise, I don't think it's worth the effort to test.

I'd be happy to work through it together. I don't think it's important to test whether window.location.reload is called, but right now it is being called during the tests and it's raising a console.error because it's not implemented in jsdom. So I think it's worth mocking just to clean up our jest output. And from there we can decide whether to go further and actually assert anything in the test.

dcdenu4

Thanks @emlys, this looks awesome! And thanks Dave for giving this a thorough look and helping debug!

dcdenu4 · 2023-01-10T15:10:34Z

Hey @emlys, I just loaded up the Workbench and tried switching to Spanish. I just had a few quick questions and sorry if they've already been addressed.

I noticed a capitalization difference in the options, "English" v "espanol". Should that be consistent?
Are we foregoing trying to translate the File | Edit | View | Window | About menus?
I noticed for a models input helper text (raster, csv, directory, number, ...) that some are translated and some are not. I was curious how that is determined. For instance, I see both tasa: un decimal de 0 - 1 and number. Just curious, thanks!

So cool to see it in action!

emlys · 2023-01-19T20:42:28Z

@davemfish I think I finally fixed the window reload mocking issue. I had to delete the object first, and also mock it in another test where the language setting changes.

davemfish · 2023-01-20T21:16:16Z

@davemfish I think I finally fixed the window reload mocking issue. I had to delete the object first, and also mock it in another test where the language setting changes.

@emlys Thanks for figuring that out! I guess window.location is kind of special in jsdom jestjs/jest#5124

emlys and others added 12 commits December 5, 2022 15:57

original spanish translation PO file

aa37149

remove dev mode requirement to show language setting

36476e6

uncomment internationalizion step of setup.py build process

aa1a2a4

include MO files in build; fix module reload in ui_server

045654f

make sure app rerenders only after localforage settings save

ccb9371

make sure validation messages rerender when language changes

b00648c

get supported languages and display names through new endpoint

36387ea

remove unused english locale directory

574a499

refresh app to change language setting

c1f63f2

mock getSupportedLanguages in tests

fb91b11

history note

7132595

Merge branch 'main' into task/spanish

c8d2f8d

emlys self-assigned this Dec 8, 2022

emlys added 5 commits December 9, 2022 16:58

update workbench POT file

3976575

Merge branch 'task/spanish' of https://github.com/emlys/invest into t…

d34e9df

…ask/spanish

fix test hanging bug

3d8393c

remove duplicate import

4782b74

add spanish message catalog for workbench messages and remove zh for now

40d216b

emlys requested review from phargogh, davemfish and dcdenu4 December 15, 2022 22:52

emlys marked this pull request as ready for review December 15, 2022 22:52

emlys changed the base branch from main to feature/spanish December 16, 2022 16:45

phargogh approved these changes Dec 19, 2022

View reviewed changes

davemfish requested changes Jan 5, 2023

View reviewed changes

emlys added 3 commits January 5, 2023 15:22

convert saveSettings function from promise chain to async/await

a178a34

update emails in PO file headers

16f8201

fix language code in spanish PO file

3722b9a

emlys requested a review from davemfish January 6, 2023 21:19

dcdenu4 approved these changes Jan 10, 2023

View reviewed changes

mock window.location.reload correctly

a28e4d8

davemfish approved these changes Jan 20, 2023

View reviewed changes

davemfish merged commit 2745b5f into natcap:feature/spanish Jan 20, 2023

emlys mentioned this pull request Jan 23, 2023

Merge Spanish translation in to 3.13 release branch #1153

Merged

3 tasks

emlys deleted the task/spanish branch March 8, 2023 01:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spanish translation #1130

Spanish translation #1130

emlys commented Dec 5, 2022 •

edited

Loading

phargogh left a comment

phargogh Dec 19, 2022

phargogh Dec 19, 2022

emlys Jan 5, 2023

phargogh Dec 19, 2022

davemfish left a comment

davemfish Jan 5, 2023

emlys Jan 5, 2023

davemfish Jan 5, 2023

emlys Jan 5, 2023 •

edited

Loading

davemfish Jan 6, 2023

emlys Jan 6, 2023

davemfish commented Jan 5, 2023

emlys commented Jan 6, 2023

davemfish commented Jan 9, 2023 •

edited

Loading

dcdenu4 left a comment

dcdenu4 commented Jan 10, 2023

emlys commented Jan 19, 2023

davemfish commented Jan 20, 2023

Spanish translation #1130

Spanish translation #1130

Conversation

emlys commented Dec 5, 2022 • edited Loading

Description

expected tasks

bug fixes

small enhancement

notes

Checklist

phargogh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davemfish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emlys Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davemfish commented Jan 5, 2023

emlys commented Jan 6, 2023

davemfish commented Jan 9, 2023 • edited Loading

dcdenu4 left a comment

Choose a reason for hiding this comment

dcdenu4 commented Jan 10, 2023

emlys commented Jan 19, 2023

davemfish commented Jan 20, 2023

emlys commented Dec 5, 2022 •

edited

Loading

emlys Jan 5, 2023 •

edited

Loading

davemfish commented Jan 9, 2023 •

edited

Loading